Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update file paths relative to input path #104

Merged
merged 1 commit into from
Feb 12, 2024
Merged

Update file paths relative to input path #104

merged 1 commit into from
Feb 12, 2024

Conversation

mehrinkiani
Copy link
Member

This PR adds attribute absolute_path to JSON report. Also makes source, scanned_files, and skipped_files fields, with file paths relative to the input path as outlined in #101.

Closes #101

Sample scan output with input path referring to a file. The model scanned is generated using XGBoost notebook.

{
    "summary": {
        "total_issues_by_severity": {
            "LOW": 0,
            "MEDIUM": 0,
            "HIGH": 0,
            "CRITICAL": 1
        },
        "total_issues": 1,
        "input_path": "notebooks/XGboostModels/unsafe_model.pkl",
        "absolute_path": "/Users/mehrinkiani/Documents/modelscan/notebooks/XGboostModels",
        "modelscan_version": "0.0.0",
        "timestamp": "2024-02-09T17:25:29.623039",
        "skipped": {
            "total_skipped": 0,
            "skipped_files": []
        },
        "scanned": {
            "total_scanned": 1,
            "scanned_files": [
                "unsafe_model.pkl"
            ]
        }
    },
    "issues": [
        {
            "description": "Use of unsafe operator 'system' from module 'posix'",
            "operator": "system",
            "module": "posix",
            "source": "unsafe_model.pkl",
            "scanner": "modelscan.scanners.PickleUnsafeOpScan",
            "severity": "CRITICAL"
        }
    ],
    "errors": []
}

Sample scan output with input path referring to a directory. The model scanned is generated using Pytorch notebook

{
    "summary": {
        "total_issues_by_severity": {
            "LOW": 0,
            "MEDIUM": 0,
            "HIGH": 0,
            "CRITICAL": 1
        },
        "total_issues": 1,
        "input_path": "notebooks/PyTorchModels",
        "absolute_path": "/Users/mehrinkiani/Documents/modelscan/notebooks/PyTorchModels",
        "modelscan_version": "0.0.0",
        "timestamp": "2024-02-09T17:24:11.058155",
        "skipped": {
            "total_skipped": 0,
            "skipped_files": []
        },
        "scanned": {
            "total_scanned": 2,
            "scanned_files": [
                "safe_model.pt:safe_model/data.pkl",
                "unsafe_model.pt:unsafe_model/data.pkl"
            ]
        }
    },
    "issues": [
        {
            "description": "Use of unsafe operator 'system' from module 'posix'",
            "operator": "system",
            "module": "posix",
            "source": "unsafe_model.pt:unsafe_model/data.pkl",
            "scanner": "modelscan.scanners.PickleUnsafeOpScan",
            "severity": "CRITICAL"
        }
    ],
    "errors": []
}

willarmiros
willarmiros approved these changes Feb 12, 2024
View reviewed changes
Copy link
Member

@willarmiros willarmiros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome, thanks for the change and examples!

A question on the 2nd output for the directory example, why is there a colon in the path below? Is this just a quirk of this particular file or is it to be expected for all files?

 "source": "unsafe_model.pt:unsafe_model/data.pkl"

@mehrinkiani
Copy link
Member Author

Thank you @willarmiros. The detailed description in the issue was helpful!

The colon in the path is used when ModelScan scans a zipped file (code reference). In the second sample output, the files safe.pt and unsafe.pt are zipped files and when unzipped, the particular file scanned is data.pkl

@willarmiros
Copy link
Member

@mehrinkiani that makes sense, sounds good and thanks for explaining!

@mehrinkiani mehrinkiani merged commit 3b59900 into main Feb 12, 2024
8 checks passed
@mehrinkiani mehrinkiani deleted the 101-change-source-file-paths branch February 12, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@willarmiros willarmiros willarmiros approved these changes

@swashko swashko Awaiting requested review from swashko

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Change source file paths to be relative to input
2 participants
@mehrinkiani @willarmiros